Skip to content

Fixed #3276 - [Content Analysis/Permalink] - Keyword with fullstop (.) is not detected in permalink because the . is converted to - in the URL#12

Open
Nickbahson wants to merge 3 commits intorankmath:masterfrom
Nickbahson:fixed-3276
Open

Fixed #3276 - [Content Analysis/Permalink] - Keyword with fullstop (.) is not detected in permalink because the . is converted to - in the URL#12
Nickbahson wants to merge 3 commits intorankmath:masterfrom
Nickbahson:fixed-3276

Conversation

@Nickbahson
Copy link
Copy Markdown

Closes #3276

@Nickbahson Nickbahson requested a review from balazspiller July 16, 2024 04:53
Copy link
Copy Markdown

@balazspiller balazspiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous version was okay too, I was just wondering why we did it that way.

BTW, on the next line: .replace( "'", '' ) only replaces the first occurrence. Is that what we want to do or is that a mistake?
And on the line after that we have .replace( /[-_]/ig, '-' ) which is also kind of strange because why do we replace - with the same - character? Shouldn't it be .replace( /[-_]+/ig, '-' ) (notice the +) so that multiple --- and ___ will be replaced by a single -?

Sorry @Nickbahson I know these are not related to your changes, but they caught my attention.

@Nickbahson Nickbahson requested a review from balazspiller July 17, 2024 10:56
@pratikrm
Copy link
Copy Markdown
Collaborator

@Nickbahson

I too think we should replace all occurrences and not just the first as mentioned by @balazsrm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants